Skip to content

Deprecate built-in CORS middleware#178

Merged
bajtos merged 1 commit into
masterfrom
feature/deprecate-cors
Sep 22, 2016
Merged

Deprecate built-in CORS middleware#178
bajtos merged 1 commit into
masterfrom
feature/deprecate-cors

Conversation

@bajtos

@bajtos bajtos commented Sep 21, 2016

Copy link
Copy Markdown
Member

Push the responsibility of enabling/configuring CORS back to the application developer.

This is a follow-up for strongloop/strong-remoting#352 and strongloop/strong-remoting#357 which removed (3.x)/deprecated (2.x) the built-in CORS middleware in strong-remoting.

@richardpringle please review

Push the responsibility of enabling/configuring CORS back to the
application developer.
Comment thread test/explorer.test.js
it('can be disabled by configuration', function(done) {
var app = loopback();
app.set('remoting', { cors: { origin: false }});
app.set('remoting', { cors: false });

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is needed to fix npm test when using LB 3.x. Before, the tests were failing because remoting.cors was rejected by LB 3.x runtime.

@richardpringle

Copy link
Copy Markdown
Contributor

Don't understand why it's necessary to test everything with cors: false. It shouldn't make a difference in the tests should it?

@bajtos

bajtos commented Sep 22, 2016

Copy link
Copy Markdown
Member Author

Don't understand why it's necessary to test everything with cors: false. It shouldn't make a difference in the tests should it?

Let's discuss here: strongloop/strong-remoting#357 (comment)

@richardpringle

Copy link
Copy Markdown
Contributor

Approved!

@bajtos
bajtos merged commit bce9bfe into master Sep 22, 2016
@bajtos
bajtos deleted the feature/deprecate-cors branch September 22, 2016 12:23
@bajtos bajtos removed the #review label Sep 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants